Conversation
Pull reviewers statsStats of the last 120 days for UWOrbital:
|
Syzygicality
left a comment
Syzygicality
left a comment
There was a problem hiding this comment.
just some minor refactoring :)
| :param filters: List of request statuses to filter by. If empty, no filtering is applied | ||
| :return: ARO requests matching the criteria | ||
| """ | ||
| with get_db_session() as session: |
There was a problem hiding this comment.
please use ARORequestWrapper found in wrappers.py to do database querying
python_test/conftest.py
Outdated
| "gs.backend.data.data_wrappers.mcc_wrappers.packet_telemetry_wrapper", | ||
| "gs.backend.data.data_wrappers.mcc_wrappers.packet_wrapper", | ||
| "gs.backend.data.data_wrappers.mcc_wrappers.telemetry_wrapper", | ||
| "gs.backend.api.v1.mcc.endpoints.aro_requests", |
There was a problem hiding this comment.
When refactoring to use the ARORequestWrapper, pls remove from this from conftest
python_test/test_aro_requests_api.py
Outdated
| @@ -0,0 +1,137 @@ | |||
| from datetime import datetime | |||
There was a problem hiding this comment.
pls adjust if necessary to accommodate the refactoring
|
also pls resolve failing pytest check |
| from fastapi import APIRouter, Query | ||
|
|
||
| from gs.backend.api.v1.mcc.models.responses import ARORequestsResponse | ||
| from gs.backend.data.data_wrappers.aro_wrapper.aro_request_wrapper import get_all_requests |
There was a problem hiding this comment.
Use the abstracted wrappers which eddie created
|
|
||
|
|
||
| @aro_requests_router.get("/", response_model=ARORequestsResponse) | ||
| async def get_aro_requests( |
There was a problem hiding this comment.
You should also define a requests base model which you pass in like this. I believe that tells fastapi to look for an incoming json object with the params defined in the base model.
There was a problem hiding this comment.
I am using query parameters to pass in the parameters currently. Does that work or should I switch to a request model?
| :param filters: List of request statuses to filter by. If empty, no filtering is applied | ||
| :return: ARO requests matching the criteria | ||
| """ | ||
| requests = get_all_requests(count, offset, filters) |
There was a problem hiding this comment.
Make sure to refer to eddies wrappers!
There was a problem hiding this comment.
Oh also forgot to mention, but if functionality of eddies wrappers are incomplete, please fill in the functionality by defining a method for said wrapper like
3 participants
Purpose
Closes #349.
Add GET endpoint for ARO requests.
New Changes
Testing
Explain tests that you ran to verify code functionality.
Outstanding Changes
If there are non-critical changes (i.e. additional features) that can be made to this feature in the future, indicate them here.